Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 86 partial sync #109

Merged
merged 2 commits into from Jul 30, 2012
Merged

Issue 86 partial sync #109

merged 2 commits into from Jul 30, 2012

Conversation

justinharding
Copy link

This change allows the sync folder to be specified on the command line using -p or --path. There are two commits. When I looked for "." in the source code I found two routines that are not called. The first commit removes that code. The second commit makes the Config class generic and moves it to libgrive so that I coud add basic unit tests, and includes the code changes to specify the sync path.

@nestal
Copy link
Member

nestal commented Jul 30, 2012

I have a few comments on this patch, but it will be easier to merge it first and then fix it afterwards.

@nestal nestal closed this Jul 30, 2012
nestal added a commit that referenced this pull request Jul 30, 2012
@nestal nestal merged commit cb60205 into Grive:master Jul 30, 2012
@nestal
Copy link
Member

nestal commented Jul 30, 2012

Justin, can you please follow the coding guideline here?

http://www.lbreda.com/grive/dev/standards

Can you please name the variables like config_file_environment_variable rather than configFileEnvironmentVariable?

@nestal
Copy link
Member

nestal commented Jul 30, 2012

It looks like you added code to allow the you user select a directory to sync other than the current directory. However, it will still be a full sync.

For partial sync I thought people mean to sync only some sub-directories in the google drive.

nestal added a commit that referenced this pull request Jul 30, 2012
nestal added a commit that referenced this pull request Jul 30, 2012
@justinharding
Copy link
Author

Thanks Nestal

Yes - I was thinking about the wording of the request after I submitted the
patch. You're right. I think this is still a useful feature so I don't
think it should be taken back out if you're happy with it - I will create a
new patch to allow a partial sync below the root folder. I see your message
referring to coding guidelines. I'll look through and adjust my styling.
Shall I amend the existing patch and submit an update with corrected
styling?

On 30 July 2012 06:12, Nestal Wan <
reply@reply.github.com

wrote:

It looks like you added code to allow the you user select a directory to
sync other than the current directory. However, it will still be a full
sync.

For partial sync I thought people mean to sync only some sub-directories
in the google drive.


Reply to this email directly or view it on GitHub:
#109 (comment)

@nestal
Copy link
Member

nestal commented Jul 31, 2012

I have already merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants